NO-JIRA: Add DevPreviewNoUpgrade as a featureset by deads2k · Pull Request #1825 · openshift/api

deads2k · 2024-03-22T19:31:37Z

Some features were getting delivered as devpreview and created their own custom gating mechanism which is not automatically tracked by our tooling. This makes it easy to add featuregates as dev preview without causing rippling impacts for all API authors.

Once we merge this, we must create a test job to show it working.

openshift-ci · 2024-03-22T19:32:01Z

Hello @deads2k! Some important instructions when contributing to openshift/api:
API design plays an important part in the user experience of OpenShift and as such API PRs are subject to a high level of scrutiny to ensure they follow our best practices. If you haven't already done so, please review the OpenShift API Conventions and ensure that your proposed changes are compliant. Following these conventions will help expedite the api review process for your PR.

deads2k · 2024-03-29T16:42:45Z

/retest

openshift-ci-robot · 2024-04-15T15:20:29Z

@deads2k: This pull request explicitly references no jira issue.

Details

In response to this:

Some features were getting delivered as devpreview and created their own custom gating mechanism which is not automatically tracked by our tooling. This makes it easy to add featuregates as dev preview without causing rippling impacts for all API authors.

Once we merge this, we must create a test job to show it working.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

JoelSpeed

Do we have some criteria by which a developer would decide if their feature is Dev vs TechPreview? And do we have somewhere where we can document that?

JoelSpeed · 2024-04-16T07:06:41Z

config/v1/types_feature.go

 	// +optional
 	// +kubebuilder:validation:XValidation:rule="oldSelf == 'CustomNoUpgrade' ? self == 'CustomNoUpgrade' : true",message="CustomNoUpgrade may not be changed"
 	// +kubebuilder:validation:XValidation:rule="oldSelf == 'TechPreviewNoUpgrade' ? self == 'TechPreviewNoUpgrade' : true",message="TechPreviewNoUpgrade may not be changed"
+	// +kubebuilder:validation:XValidation:rule="oldSelf == 'DevPreviewNoUpgrade' ? self == 'DevPreviewNoUpgrade' : true",message="DevPreviewNoUpgrade may not be changed"


I don't remember there being CEL here, but, LGTM. Is it new?

I don't remember there being CEL here, but, LGTM. Is it new?

"while you were out" :)

JoelSpeed · 2024-04-16T07:09:37Z

tools/codegen/cmd/featureset-markdown.go

 				counts[featureGate] = counts[featureGate] + 1
 			}
+			for _, featureGate := range byFeature.disabled.List() {
+				counts[featureGate] = counts[featureGate] + 0


Ineffective assignment? Did you mean add 0?

Ineffective assignment? Did you mean add 0?

I think so. Just making it clear how to bump in the list and nice and parallel. Similar blocks exist in other repos that do shift priority.

JoelSpeed · 2024-04-16T07:10:47Z

tools/codegen/pkg/manifestmerge/generator.go

-		crdFilename := strings.ReplaceAll(crdFilenamePattern, "MARKERS", fmt.Sprintf("-%s", utils.ClusterProfileToShortName(clusterProfile)))
+		clusterProfileShortName, err := utils.ClusterProfileToShortName(clusterProfile)
+		if err != nil {
+			panic(fmt.Sprintf("unrecognized clusterprofile name %q: %w", clusterProfile, err))


Why is this a panic rather than failing gracefully? Is this user in put or a code problem?

JoelSpeed · 2024-04-16T07:11:05Z

tools/codegen/pkg/manifestmerge/generator.go

-		crdFilename := strings.ReplaceAll(crdFilenamePattern, "MARKERS", fmt.Sprintf("-%s-%s", utils.ClusterProfileToShortName(curr.clusterProfile), curr.featureSet))
+		clusterProfileShortName, err := utils.ClusterProfileToShortName(curr.clusterProfile)
+		if err != nil {
+			panic(fmt.Sprintf("unrecognized clusterprofile name %q: %w", curr.clusterProfile, err))


As above, should this really be a panic?

deads2k · 2024-04-17T13:19:39Z

/retest-required

JoelSpeed · 2024-04-17T13:24:36Z

/lgtm

openshift-ci · 2024-04-17T13:38:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, JoelSpeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [JoelSpeed,deads2k]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci-robot · 2024-04-17T13:45:13Z

/retest-required

Remaining retests: 0 against base HEAD 3efee42 and 2 for PR HEAD 17878f0 in total

deads2k · 2024-04-17T14:01:08Z

/retest

deads2k · 2024-04-17T15:52:53Z

/retest

deads2k · 2024-04-17T20:57:49Z

/retest

openshift-ci · 2024-04-17T23:08:33Z

@deads2k: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2024-04-18T16:46:17Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cluster-config-api-container-v4.16.0-202404181209.p0.g21822c8.assembly.stream.el9 for distgit ose-cluster-config-api.
All builds following this will include this PR.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 22, 2024

openshift-ci bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 22, 2024

openshift-ci bot requested review from cheesesashimi and danwinship March 22, 2024 19:33

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 22, 2024

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 23, 2024

deads2k force-pushed the by-gate-27-dev-preview branch from 7f2ce3c to 6bfc356 Compare March 29, 2024 14:26

deads2k changed the title ~~[wip] starting devpreview discussions~~ Add DevPreviewNoUpgrade as a featureset Mar 29, 2024

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 29, 2024

deads2k force-pushed the by-gate-27-dev-preview branch from 6bfc356 to 3ede02b Compare March 29, 2024 14:29

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 29, 2024

deads2k force-pushed the by-gate-27-dev-preview branch from 3ede02b to 28f074c Compare March 29, 2024 15:17

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2024

deads2k changed the title ~~Add DevPreviewNoUpgrade as a featureset~~ NO-JIRA: Add DevPreviewNoUpgrade as a featureset Apr 15, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Apr 15, 2024

deads2k force-pushed the by-gate-27-dev-preview branch from 28f074c to 54c24d1 Compare April 15, 2024 15:35

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 15, 2024

deads2k force-pushed the by-gate-27-dev-preview branch 2 times, most recently from b18f8d8 to 27c4944 Compare April 15, 2024 22:07

JoelSpeed reviewed Apr 16, 2024

View reviewed changes

deads2k force-pushed the by-gate-27-dev-preview branch from 27c4944 to cc7b693 Compare April 16, 2024 15:37

add perma-disabled featuregates to markdown

7814637

deads2k force-pushed the by-gate-27-dev-preview branch from cc7b693 to 9ab6ed1 Compare April 16, 2024 17:57

add DevPreviewNoUpgrade featureset

c43b32d

deads2k force-pushed the by-gate-27-dev-preview branch 2 times, most recently from 774b4ca to de5d2ab Compare April 16, 2024 21:27

update tooling to auto-find new featuresets

ef21ee7

gracefully return generator errors

17878f0

deads2k force-pushed the by-gate-27-dev-preview branch from de5d2ab to 17878f0 Compare April 17, 2024 12:55

openshift-ci bot assigned JoelSpeed Apr 17, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Apr 17, 2024

openshift-merge-bot bot merged commit 21822c8 into openshift:master Apr 17, 2024

Miciah mentioned this pull request Apr 24, 2024

OCPBUGS-31666: route: Fix insecureEdgeTerminationPolicy default #1845

Merged

deads2k mentioned this pull request Apr 29, 2024

NO-JIRA: Allow DevPreviewNoUpgrade openshift/installer#8327

Closed

Conversation

deads2k commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openshift-ci bot commented Mar 22, 2024

Uh oh!

deads2k commented Mar 29, 2024

Uh oh!

openshift-ci-robot commented Apr 15, 2024

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

deads2k Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

deads2k Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

JoelSpeed Apr 16, 2024

Choose a reason for hiding this comment

Uh oh!

deads2k commented Apr 17, 2024

Uh oh!

JoelSpeed commented Apr 17, 2024

Uh oh!

openshift-ci bot commented Apr 17, 2024

Uh oh!

openshift-ci-robot commented Apr 17, 2024

Uh oh!

deads2k commented Apr 17, 2024

Uh oh!

deads2k commented Apr 17, 2024

Uh oh!

deads2k commented Apr 17, 2024

Uh oh!

openshift-ci bot commented Apr 17, 2024

Uh oh!

openshift-bot commented Apr 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

deads2k commented Mar 22, 2024 •

edited

Loading